Fix for check-manifest pre-commit hook in Python 3.12 #344
Conversation
sfmig
changed the title
|
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #344 +/- ##
=======================================
Coverage 99.77% 99.77%
=======================================
Files 14 14
Lines 907 907
=======================================
Hits 905 905
Misses 2 2 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
Thanks for this @sfmig!
Good to go as is, regarding your questions I don't have informed opinions, but here are my 2 cents anyway:
Right now, when we say we support Python 3.12 in practice we mean we run the code tests in Python 3.12.
I think that's what most Python projects mean, right?
But should we ensure other bits, like the developer tools, also run on the Python versions we support?
I'd say, yes. Our current docs for a development environment recommend Python 3.11, just because we tend to go for the "middle" one of the 3 supported versions at any given point, out of convention. That said, I'd say it's a reasonable expectation that dev would work for all of our supported Python versions (hence why this PR is good).
If so, maybe it makes sense if we ensure that the CI actions related to precommits run the latest python version?
Maybe, to catch cases like this one? But where would you make this change, here or in the actions repo? The problem with changing it in actions is that projects that rely on those may differ in their supported Python versions?
|
@sfmig is this ready to be merged? |
|
yes! Apologies, I think I will open a PR in the actions repo / ask other team members tomorrow about this (but that belongs to the other repo) |
github-merge-queue
bot
removed this pull request from the merge queue due to failed status checks
* update project overview * update mission statement * updated scope * update roadmaps and consistently use `movement` (monospace) * Add wheel as a dependency (#344) * implement Adam's suggestions * Apply some suggestions outright Co-authored-by: sfmig <[email protected]> * [pre-commit.ci] auto fixes from pre-commit.com hooks for more information, see https://pre-commit.ci * update project overview based on feedback * clarify statement about action recognition * updated scope * mention "keypoints" for SLEAP and DLC representations in "scope". --------- Co-authored-by: sfmig <[email protected]> Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
Description
What is this PR
Why is this PR needed?
With Python 3.12,
pre-commit run check-manifestreturns an error of missing dependencies -- see more details in this issue.To fix it,
wheelneeds to be specified as a dependency ofcheck-manifestin the pre-commit config.This error doesn't show up in CI because:
check-manifestwith Python 3.10 and without the--no-build-isolationflag.Question
What does this PR do?
Adds
wheelas a dependency tocheck-manifestin the pre-commit config.References
The most relevant is this issue. from the cookiecutter repo.
How has this PR been tested?
Tests pass locally and in CI.
Is this a breaking change?
No.
Does this PR require an update to the documentation?
No.
Checklist: